feat: constrain call by IlyasRidhuan · Pull Request #13758 · AztecProtocol/aztec-packages

IlyasRidhuan · 2025-04-23T10:30:25Z

Begin constraining call. Limited PIL relations since we are still missing the infrastructure for transaction trace.

PIL relations

Add context switching relations to either propagate context state during execution and updating context on CALL

Simulation

Updated the call operation in Execution to handle additional parameters for L2 and DA gas offsets. These values and contract address are assigned to their respective execution registers.
New getter for next_context_id to populate field execution event (we need this while we aren't using clk for context id)

Context Serialization and Event Updates:

ContextEvent includes parent_id and next_pc fields (we'll need this for JUMP commands as well).
Modified serialize_context_event methods in EnqueuedCallContext and NestedContext to populate new fields like parent_id and next_pc.

Testing:

There are updated constraining, simulation and tracegen tests

IlyasRidhuan · 2025-04-23T10:30:43Z

feat: constrain call #13758 👈 (View in Graphite)
feat: exec opcode spec table #13594
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

IlyasRidhuan · 2025-04-24T16:21:44Z

barretenberg/cpp/src/barretenberg/vm2/simulation/context.hpp

-                 .last_child_success = get_last_success() };
+        return {
+            .id = get_context_id(),
+            .parent_id = 0,


this is the main thing that changed (besides formatting)

IlyasRidhuan · 2025-04-24T16:21:59Z

barretenberg/cpp/src/barretenberg/vm2/simulation/context.hpp

-                 .parent_cd_size_addr = parent_cd_size };
+        return {
+            .id = get_context_id(),
+            .parent_id = parent_context.get_context_id(),


fcarreiro

Left some comments. Could you remind me if we already have a context stack and are we constraining the snapshotting?

fcarreiro · 2025-04-24T17:42:07Z

barretenberg/cpp/pil/vm2/context.pil


+    // Useful to define some opcodes within this pil file
+    // Constrained to be boolean by execution instruction spec table
+    pol commit call_sel;


didn't we just discuss _sel versus sel_ :P

maybe we want sel_op_.... ?

hmm ok i get confused by our naming conventions..i dont have a strong opinion outside of it being easy for me to remember the convention 😅 .

We have some that are _sel (i.e {namespace}_sel) for the main selectors in each subtraces - these kinda map to opcodes.
I've used sel_ (e.g. sel_alu, sel_bitwise) for selectors that weren't "opcodes".

I'd say we use sel_ everywhere for everything that is a selector, and after that you can add what you want to make it clear that it is or not an opcode. Easy to remember! always sel_ ;)

BTW when I say they all start with sel, I mean in PIL. Of course in tracegen etc you'll have them all prefixed by namespace.

barretenberg/cpp/pil/vm2/context.pil

fcarreiro · 2025-04-24T17:51:03Z

barretenberg/cpp/src/barretenberg/vm2/simulation/execution_components.hpp

    virtual std::unique_ptr<AddressingInterface> make_addressing(AddressingEvent& event) = 0;
+
+    // This can be removed if we use clk for the context id
+    virtual uint32_t get_next_context_id() = 0;


Why in the execution components? Did you consider some other place?

The main reason it's here is that the actual id is maintained in execution_components it seemed easier to do it here. next_context_id needs to "live" somewhere that persists at a transaction level since it's used by the transaction trace as well.

At a stretch this could be left to the tracegen-only, but it's complicated as it's value will need to be shared across execution & transaction traces so communicating via the events would be easier

Makes sense. Probably the contextprovider would own it, but that was converted to the execution components :)

fcarreiro · 2025-04-24T17:51:10Z

barretenberg/cpp/src/barretenberg/vm2/simulation/execution_components.hpp

+
  private:
-    uint32_t next_context_id = 0;
+    uint32_t next_context_id = 1; // 0 is reserved to denote the parent of a top level context


Who ever increments this?

make_nested_context and make_enqueued_context

But they aren't now are they? Probably should be done in this PR. Or maybe I'm missing sth

it should do now via uint32_t context_id = next_context_id++; in those functions.

I dont think we test this yet though..

fcarreiro · 2025-04-24T17:52:36Z

barretenberg/cpp/src/barretenberg/vm2/simulation/events/execution_event.hpp

    std::vector<TaggedValue> output;

+    // Context Id for the next context.
+    uint32_t next_context_id;


Why do we need, for every event, the NEXT context id? Why not the current and why an id at all?

(I'm not asking for a change, but an explanation :) )

Ah good question (this holds for enqueued calls but it's easier just to consider nested calls for now).

The current id is stored within the context event - it's needed to uniquely identify a context when interacting with the stack AND it is used as the space_id for memory (we should really deprecate the use of space_id IMO).

When we encounter a call we need to assign a new unique id to the new context. We cannot just increment the current id because we could have hit a RETURN and are actually in a prior context.

Start in initial context: ID = 1

Run CALL, creating new context with ID = 2

Return from ID=2 back to context ID = 1

A new context from a new CALL/enqueued call would need to be assigned ID = 3

If we just incremented the current ID = 1, we would get a duplicate context with ID = 2. In order to know that we the next ID is 3 we have to track it at a global level (which is why this doesnt live in context) and this was the easiest way i could think of - maybe we could do some trick with the stack?

ah I get it, it's the "next available context id". I thought it was something weird like "ok this is the context id of the call i'm about to go in to". Consider name change (or comment) but not needed.

fcarreiro · 2025-04-24T18:05:17Z

barretenberg/cpp/pil/vm2/context.pil


+    // Useful to define some opcodes within this pil file
+    // Constrained to be boolean by execution instruction spec table
+    pol commit call_sel;


Won't you also have this in the execution trace eventually?

this is actually the one from the execution trace..but you are right that perhaps i should move this to the execution.pil rather than this virtual trace.

I had it in here because my intuition is that most of the relations involving call_sel will be in this file.

barretenberg/cpp/pil/vm2/context.pil

fcarreiro · 2025-04-25T12:32:05Z

barretenberg/cpp/pil/vm2/execution.pil


 pol commit sel; // subtrace selector
+
+#[skippable_if]


ohh we didn't have this

fcarreiro · 2025-04-25T14:57:17Z

barretenberg/cpp/src/barretenberg/vm2/simulation/events/execution_event.hpp

    std::vector<TaggedValue> output;

+    // Context Id for the next context.
+    uint32_t next_context_id;


ah I get it, it's the "next available context id". I thought it was something weird like "ok this is the context id of the call i'm about to go in to". Consider name change (or comment) but not needed.

fcarreiro · 2025-04-25T14:57:46Z

barretenberg/cpp/src/barretenberg/vm2/simulation/execution_components.hpp

    virtual std::unique_ptr<AddressingInterface> make_addressing(AddressingEvent& event) = 0;
+
+    // This can be removed if we use clk for the context id
+    virtual uint32_t get_next_context_id() = 0;


Makes sense. Probably the contextprovider would own it, but that was converted to the execution components :)

This pr got a little bit oversized in complexity and scope. It essentially adds the "exit" version of the #13758, adding context-level constraints for return/revert and error (this the "execution-level" error which should consolidate any errors propagated from subtraces). Success will be handled in the next pr...hopefully ### Circuit * The circuit likely has more columns than it needs, but this can be re-visited or optimised later. * New selectors (`sel_enter_call`, `sel_exit_call`, `sel_revert`, `sel_return`, etc.) and constraints to handle context switching - namely for nested calls `context.pil`. There are additionally columns to specify returns from contexts that have a parent (nested context). * Returndata information is constrained when exiting - (it is likely incorrect for error but it is TBD whether this matters if the error flag conditions it's use in the parent context during returndatacopy etc) * `context_stack.pil` has an actual constraint for its selector. * Added `instr_length` column in `execution.pil` to increment `next_pc`. The lookup to the instr_fetching is still pending. ### Simulation * Added a new `revert` method in the `Execution` class to handle revert operations, ensuring proper state rollback during errors. * The `ExecutionEvent` now contains an `error` field, but it's not populated through execution yet. ### Testing * Bunch of new tests (although still not enough). The main one is the updating of the `constraining` tests that include the lookup check and a simple CALL -> RETURN context switch --------- Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com>

IlyasRidhuan mentioned this pull request Apr 23, 2025

feat: exec opcode spec table #13594

Merged

IlyasRidhuan force-pushed the ir/04-14-feat_constrain_call branch from c36fdc9 to 509c85d Compare April 23, 2025 10:37

IlyasRidhuan force-pushed the ir/04-14-feat_exec_opcode_spec_table branch from f15395f to d09effe Compare April 23, 2025 12:58

IlyasRidhuan force-pushed the ir/04-14-feat_constrain_call branch from 509c85d to 930490a Compare April 23, 2025 12:58

Base automatically changed from ir/04-14-feat_exec_opcode_spec_table to master April 23, 2025 16:07

IlyasRidhuan force-pushed the ir/04-14-feat_constrain_call branch from 930490a to b3f4a96 Compare April 24, 2025 12:53

feat: constrain call

6c09a2f

IlyasRidhuan force-pushed the ir/04-14-feat_constrain_call branch from b3f4a96 to 6c09a2f Compare April 24, 2025 14:07

feat: update context stack

a06524c

IlyasRidhuan requested review from dbanks12 and fcarreiro April 24, 2025 15:35

IlyasRidhuan marked this pull request as ready for review April 24, 2025 15:35

IlyasRidhuan requested review from Maddiaa0 and jeanmon as code owners April 24, 2025 15:35

IlyasRidhuan removed request for Maddiaa0 and jeanmon April 24, 2025 15:36

IlyasRidhuan commented Apr 24, 2025

View reviewed changes

fcarreiro reviewed Apr 24, 2025

View reviewed changes

address comments

027e713

IlyasRidhuan requested a review from fcarreiro April 25, 2025 12:21

fcarreiro approved these changes Apr 25, 2025

View reviewed changes

IlyasRidhuan added this pull request to the merge queue Apr 25, 2025

Merged via the queue into master with commit 7d7990e Apr 25, 2025
8 checks passed

IlyasRidhuan deleted the ir/04-14-feat_constrain_call branch April 25, 2025 16:19

AztecBot mentioned this pull request Apr 25, 2025

chore(master): release 0.87.0 #13800

Merged

IlyasRidhuan mentioned this pull request Apr 29, 2025

feat: return, revert, halt, and context stack lookups #13912

Merged

Conversation

IlyasRidhuan commented Apr 23, 2025 • edited by fcarreiro Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PIL relations

Simulation

Context Serialization and Event Updates:

Testing:

Uh oh!

IlyasRidhuan commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fcarreiro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IlyasRidhuan commented Apr 23, 2025 •

edited by fcarreiro

Loading

IlyasRidhuan commented Apr 23, 2025 •

edited

Loading